Skip to content

Reject a mode change while an object is incomplete - #736

Draft
AbhinavMir wants to merge 1 commit into
msgpack:mainfrom
AbhinavMir:unpacker-skip-mode-switch
Draft

AbhinavMir wants to merge 1 commit into
msgpack:mainfrom
AbhinavMir:unpacker-skip-mode-switch

Conversation

@AbhinavMir

@AbhinavMir AbhinavMir commented Aug 30, 2026 •

Copy link
Copy Markdown

Unpacker.skip() does not build the objects on the parser stack. So
stack[].obj stays uninitialized after an incomplete skip(). A later
Unpacker.unpack() writes an array item through that pointer and the
process crashes. The reverse order drops the reference that the
construct pass took.

Record the mode that started the object in unpack_context.
unpack_construct() and unpack_skip() now raise ValueError on a change.
The parser state stays intact, so the original mode still finishes the
object.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The guard preserves parser state and the tests cover both unsafe mode-switch directions.

Pull request overview

Prevents unsafe switching between Unpacker.unpack() and skip() while parsing an incomplete container.

Changes:

  • Tracks the active parser construction mode.
  • Rejects incompatible mode switches with ValueError.
  • Adds regression coverage for both switch directions and state recovery.
File summaries
File Description
msgpack/unpack_template.h Adds mode tracking and validation.
test/test_sequnpack.py Tests rejected mode switches and subsequent completion.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants